Rename the implicit result variable along with its function - #538
Rename the implicit result variable along with its function#538kunalKumar-13 wants to merge 1 commit into
Conversation
A function declared without a RESULT() clause returns through an implicit result variable that shares the function's name. fortls models it as a distinct object with its own FQSN -- `mod::fun` for the function and `mod::fun::fun` for the result variable -- so the equality test in get_all_references never matched it and textDocument/rename left the assignment in the body untouched, producing code that no longer compiles. Link the two through the existing override_cache, in both directions, so a rename started from either the declaration or the body updates both. This only applies when the result variable is implicit; functions with an explicit RESULT() clause are unaffected, and intrinsics are still skipped. textDocument/references and documentHighlight share this code path and are fixed by the same change. Fixes fortran-lang#322
There was a problem hiding this comment.
Pull request overview
This pull request fixes Fortran symbol rename behavior for functions declared without an explicit RESULT() clause, ensuring the implicit result variable in the function body is renamed together with the function (addressing #322). This aligns textDocument/rename behavior with textDocument/definition and also corrects textDocument/references / documentHighlight since they share the same reference-resolution path.
Changes:
- Link a function and its implicit result variable bidirectionally during reference collection so both are treated as rename targets.
- Add a new Fortran fixture covering implicit-result and explicit-
RESULT()function cases. - Add three rename tests (rename from declaration, rename from body, and explicit
RESULT()non-regression) and document the fix in the changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
fortls/langserver.py |
Extends get_all_references to treat a function and its implicit result variable as linked rename/references targets via override_cache. |
test/test_source/rename/test_rename_implicit_result.f90 |
Adds a fixture module demonstrating implicit result assignment vs explicit RESULT() behavior. |
test/test_server_rename.py |
Adds regression tests for renaming implicit function results (from declaration and from body) and a guard for explicit RESULT() behavior. |
CHANGELOG.md |
Records the fix under Unreleased. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The red X here is not this change. All 8 failing jobs fail at the same single step, I re-ran the CI sequence locally against this branch on Python 3.13 to be sure the change itself is clean: Nothing needed from me here as far as I can tell, but happy to rebase if that would help. |
|
The red builds here are not from this change. On every failing job the only failing step is Upload coverage to Codecov — It is repo-wide rather than specific to this PR: the most recent Happy to rebase once CI is healthy if that would help. |
Fixes #322
The bug
A Fortran function declared without a
RESULT()clause returns through animplicit result variable that shares the function's name:
Renaming
sindrenamed only the declaration and left the assignment alone,producing code that no longer compiles.
Root cause
fortls models the implicit result variable as a distinct object with its own
FQSN. For the function above:
function sind(x)Functionm::sindsind = ...Variablem::sind::sindget_all_referencesmatches candidates withdef_fqsn == var_def.FQSN(
langserver.py:994), so the body occurrence never matched and was dropped.Two things worth noting, since the issue speculated about both:
myfuncfails identically, sosindshadowing the intrinsic is notinvolved. After this change the original
sindcase renames correctly andthe
sin/atancalls on the same line are still left alone.RESULT()clause were never affected —there both sites resolve to the same
Variable, so they already matched.textDocument/definitionalready resolved the body occurrence to thefunction, so rename and definition disagreed with each other.
The fix
Link the function and its implicit result variable through the existing
override_cache, in both directions, so a rename started from thedeclaration or from the body updates both sites. The link is only made when
result_name == name, i.e. when the result variable really is implicit;explicit
RESULT()clauses take the existing path untouched.textDocument/referencesanddocumentHighlightshare this code path and arefixed by the same change:
Tests
Three tests plus a new fixture,
test_rename_implicit_result.f90:test_rename_implicit_function_result— rename from the declarationtest_rename_implicit_function_result_from_body— rename from the bodytest_rename_explicit_function_result— explicitRESULT()unchangedThe first two fail on
masterand pass here; the third passes either way andguards against a regression.
They assert the number of changes explicitly, because
check_rename_responsezips changes against expectations and would otherwisesilently pass when a change is missing — which is exactly the failure mode
here.
Full suite: 184 passed.
pre-commitclean.